ci: give rustdoc its own job instead of burying it in test - #3
Merged
Conversation
A commit with a broken intra-doc link reached the PR branch last cycle. I said CI would have caught it; checking the run history, that was wrong in an instructive way -- the run on that commit is marked CANCELLED, not failed. rustdoc never executed. The gate did not fail, it did not run. Two causes, both structural rather than bad luck: 1. rustdoc was the LAST step of the `test` job, sitting behind fmt, clippy and the full test run. With `cancel-in-progress` enabled for pull_request events (a deliberate cost optimisation), a quick follow-up push supersedes the run -- and whatever is furthest from the front of the queue is what gets lost. Being last made it the most likely casualty. 2. When it did fail, it reported as "test (ubuntu-latest) failed", which points at the wrong subsystem. Nothing in the check name said "docs". Now its own job: `rustdoc (-D warnings)`, ubuntu-only, with no `needs:` so it starts immediately rather than queueing behind the matrix. It finishes in well under a minute -- fast enough to be genuinely useful feedback, and small enough that it is rarely mid-flight when a supersede happens. The job name now names what broke. The comment in the workflow records WHY it is a separate job, so a future tidy-up does not fold it back into `test` as an apparent simplification. Verified rather than assumed: reintroducing the exact defect that slipped through (public docs linking to the private `Pipeline::flush_pending`) makes the new job's command fail, while `cargo test --workspace` stays green and blind to it -- which is precisely why it needed to stop being a step inside the test job. Note this does not change what `main` is verified against: cancel-in-progress only applies to pull_request events, and the post-merge run on `main` always executes in full. What was at risk was catching the problem EARLY, on the PR, which is the entire point of running it there. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens the CI gate for documentation by moving cargo doc (with -D warnings) into its own fast, ubuntu-only job so rustdoc failures aren’t masked by cancel-in-progress cancellations and show up with a dedicated check name.
Changes:
- Split rustdoc out of the
testjob into a standalonerustdoc (-D warnings)job that starts immediately (noneeds:). - Document the rationale for the separation in the workflow file to prevent accidental re-bundling later.
- Record the CI structural fix in the Unreleased changelog.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
CHANGELOG.md |
Adds an Unreleased “Fixed” entry describing why rustdoc was split into its own CI job. |
.github/workflows/ci.yml |
Introduces a dedicated rustdoc (-D warnings) job running cargo doc --workspace --no-deps in parallel with tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the CHANGELOG.md to document a CI improvement where the rustdoc check was moved into its own dedicated job. This change ensures that rustdoc runs in parallel with tests and is not skipped due to cancel-in-progress events. There are no review comments to evaluate, and the changes are appropriate.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Adopts Copilot's comment on PR #3. The entry abbreviated the command to `RUSTDOCFLAGS="-D warnings" cargo doc`, dropping `--workspace --no-deps`. Harmless as prose, but the entry is describing a CI gate, so it reads as a local repro -- and the abbreviated form behaves differently: without `--no-deps` it documents every dependency too, so `-D warnings` can fail on third-party doc warnings the project does not control. Someone copying it to reproduce a CI failure could land on a confusing false positive. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
doublegate
added a commit
that referenced
this pull request
Jul 22, 2026
Adjudicating the PR #56 bot review: - **Wait for the whole command** (CodeRabbit #2, Antigravity blocking #2): `Rdp::tick` now returns without advancing when `DPC_END - DPC_CURRENT` is less than the decoded length. The rdpq microcode advances `DPC_END` incrementally as it fills the buffer, so it can land mid-command; consuming a partial multi-word primitive would decode unwritten RDRAM. New test drives a 22-word triangle with `DPC_END` first at word 10 (stalls) then at word 22 (consumed whole). Mutation-checked. - **XBUS stalls the decoder** (CodeRabbit #3): with `DPC_STATUS.XBUS` set the command source is DMEM, which is not yet wired, so the decoder must not fall back to reading RDRAM (that would decode parameter data as opcodes and desync). New test asserts no advance under XBUS. Mutation-checked. - **Independent decoder test** (CodeRabbit #1): the FIFO-walk fixture now states each command's word count explicitly rather than calling `command_len_words`, so the walk is a genuine check of the decoder against the N64brew map, not a tautology built from it. Rejected: Antigravity blocking #1 (texture rectangle is 3 words). The N64brew command map shows 0x24/0x25 as two 64-bit words (Word 0 coords + Word 1 s/t/dsdx/dtdy); there is no third dsdy/dtdx word — texrect is axis-aligned and carries only dsdx and dtdy. The decoder's 2 stands. Docs: docs/rdp.md gains the two stall conditions; CHANGELOG updated. Gates: fmt, clippy --workspace -D warnings, cargo test --workspace, rustdoc -D warnings, thumbv7em no_std, markdownlint — all green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
doublegate
added a commit
that referenced
this pull request
Jul 24, 2026
Adjudicates the Stage A review. CodeRabbit correctly found a stale scalar-only claim, but its suggested direction (revert to scalar-only) is wrong: the VU runs. Rsp::tick -> su_step dispatches COP2 (cop2 -> vu_compute/vu_single_lane) and the vector load/store family, and the n64-systemtest RSP category is Failed: 0. Fixed the stale comments to match (the correct direction): - rsp/src/lib.rs Rsp::tick: reworded to state su_step runs the full scalar+vector engine (was 'the vector unit is Sprint 2; the scalar unit runs today'). - su.rs: the COP2 dispatch comment + the cop2() doc no longer say 'computational instructions to come' -- they dispatch to the VU. - STATUS.md: cite the systemtest runner on the COP1 Failed:0 claim (CR #2). - VERSION-PLAN.md: 'seven feature releases (plus v0.4.1 doc patch)' (CR #3). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem, as observed
A commit with a broken intra-doc link reached the PR branch last cycle. I claimed CI would have caught it. Checking the run history, that was wrong in an instructive way — the run on that commit is marked
cancelled, not failed. rustdoc never executed. The gate did not fail; it did not run.Two structural causes:
testjob, behind fmt, clippy and the full test run. Withcancel-in-progresson forpull_requestevents (a deliberate cost optimisation), a quick follow-up push supersedes the run — and whatever is furthest back in the queue is what gets lost. Being last made it the likeliest casualty.test (ubuntu-latest) failed, pointing at the wrong subsystem.The fix
A dedicated
rustdoc (-D warnings)job, ubuntu-only, with noneeds:so it starts immediately instead of queueing behind the matrix. Sub-minute, so it's useful feedback and rarely mid-flight when a supersede happens. The check name now names what broke.The workflow comment records why it's separate, so a future tidy-up doesn't fold it back into
testas an apparent simplification.Verified, not assumed
Reintroduced the exact defect that slipped through (public docs linking to private
Pipeline::flush_pending):RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-depscargo test --workspaceThat second row is exactly why it needed to stop being a step inside the test job.
Scope note
This does not change what
mainis verified against —cancel-in-progressapplies only topull_requestevents, and the post-merge run onmainalways executes in full. What was at risk was catching the problem early, on the PR, which is the whole point of running it there.actionlintclean.